HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet#3065
Merged
ferhui merged 9 commits intoapache:trunkfrom Jun 17, 2021
Merged
HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet#3065ferhui merged 9 commits intoapache:trunkfrom
ferhui merged 9 commits intoapache:trunkfrom
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
ferhui
reviewed
Jun 2, 2021
| blockManager.getDatanodeManager().getDatanodes()) { | ||
| for (long ucFileId : dataNode.getLeavingServiceStatus().getOpenFiles()) { | ||
| // Sort open files | ||
| LightWeightHashSet<Long> dnOpenFiles = dataNode.getLeavingServiceStatus().getOpenFiles(); |
Contributor
|
Failed tests pass locally. Now it is already for review! |
xiaoyuyao
reviewed
Jun 2, 2021
| public DatanodeCommand blockReport(DatanodeRegistration registration, | ||
| String poolId, StorageBlockReport[] reports, | ||
| BlockReportContext context) | ||
| String poolId, StorageBlockReport[] reports, BlockReportContext context) |
Contributor
There was a problem hiding this comment.
NIT: unnecessary formatting change can be avoided.
|
💔 -1 overall
This message was automatically generated. |
aajisaka
reviewed
Jun 8, 2021
Member
aajisaka
left a comment
aajisaka
left a comment
There was a problem hiding this comment.
Now I've reviewed 35/38 files and the patch mostly looks good to me. I'll review the rest.
...op-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java
Outdated
Show resolved
Hide resolved
...op-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeStorageInfo.java
Outdated
Show resolved
Hide resolved
...c/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockHasMultipleReplicasOnSameDN.java
Outdated
Show resolved
Hide resolved
...adoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java
Outdated
Show resolved
Hide resolved
...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Show resolved
Hide resolved
...oop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaMap.java
Outdated
Show resolved
Hide resolved
aajisaka
reviewed
Jun 8, 2021
Member
aajisaka
left a comment
aajisaka
left a comment
There was a problem hiding this comment.
Reviewed all the files. Thank you!
...ct/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
Outdated
Show resolved
Hide resolved
ferhui
reviewed
Jun 8, 2021
...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Show resolved
Hide resolved
Contributor
Author
|
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
reviewed
Jun 11, 2021
Contributor
xiaoyuyao
left a comment
xiaoyuyao
left a comment
There was a problem hiding this comment.
I added a few comments inline. But the change looks mostly good to me. Thanks for the work.
|
|
||
| DatanodeDescriptor dn = storageInfo.getDatanodeDescriptor(); | ||
|
|
||
| LOG.debug("Reported block {} on {} size {} replicaState = {}", block, dn, |
Contributor
There was a problem hiding this comment.
This needs to be wrapped with if (LOG.isDebugEnabled()) as toString() of dn and block can be expensive.
Member
There was a problem hiding this comment.
If the log level is not debug, toString() is not actually called.
http://www.slf4j.org/faq.html#logging_performance
Contributor
Author
There was a problem hiding this comment.
i have changed it back, removing if (LOG.isDebugEnabled())
| // against the memory state. | ||
| // See HDFS-6289 and HDFS-15422 for more context. | ||
| queueReportedBlock(storageInfo, replica, reportedState, | ||
| queueReportedBlock(storageInfo, block, reportedState, |
Contributor
There was a problem hiding this comment.
block should be storedBlock?
Contributor
Author
There was a problem hiding this comment.
@xiaoyuyao you are right, it should be storedBlock
Contributor
There was a problem hiding this comment.
I believe this is now correct, as it should be the reported block queue, rather than the stored block.
Contributor
Author
|
@xiaoyuyao Thanks for review, i have update this PR, take a look please. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
@xiaoyuyao Thanks for review! Is @AlphaGouGe 's fix OK? |
sodonnel
reviewed
Jun 15, 2021
| // against the memory state. | ||
| // See HDFS-6289 and HDFS-15422 for more context. | ||
| queueReportedBlock(storageInfo, replica, reportedState, | ||
| queueReportedBlock(storageInfo, storedBlock, reportedState, |
Contributor
There was a problem hiding this comment.
I this this change is incorrect - we should be queuing the details reported in the IBR, which I think is "replica" here. This change was made in HDFS-15422.
Contributor
There was a problem hiding this comment.
That's right. This will undo the fix.
Contributor
Author
There was a problem hiding this comment.
sorry for missing the change on HDFS-15422, i have changed it back.
Contributor
|
I think the patch looks good except for the things that were already pointed out by others. |
xiaoyuyao
reviewed
Jun 15, 2021
Contributor
xiaoyuyao
left a comment
xiaoyuyao
left a comment
There was a problem hiding this comment.
LGTM, a few more comments added.
Contributor
Author
|
Thanks @kihwal @xiaoyuyao @sodonnel for review, i have updated the PR, take a look please. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
@AlphaGouGe Thanks for contribution. @aajisaka @sodonnel @kihwal @xiaoyuyao Thanks for careful review! All review comments are addressed, now it is ready to merge |
ferhui
pushed a commit
to ferhui/hadoop
that referenced
this pull request
Jun 17, 2021
…#removeAndGet (apache#3065) Conflicts: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
ferhui
pushed a commit
to ferhui/hadoop
that referenced
this pull request
Jun 17, 2021
…#removeAndGet (apache#3065) Conflicts: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DirectoryScanner.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/ReplicaMap.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockInfo.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
kiran-maturi
pushed a commit
to kiran-maturi/hadoop
that referenced
this pull request
Nov 24, 2021
Xushaohong
pushed a commit
to Xushaohong/hadoop
that referenced
this pull request
Jul 20, 2022
…oldedTreeSet#removeAndGet (apache#3065) (merge request !395) Squash merge branch 'THADOOP-187' into 'release-3.2.1-tq-0.2' * add UT testBlockListMoveToHead * HDFS-13671. Namenode deletes large dir slowly caused by FoldedTreeSet#removeAndGet (apache#3065)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/HDFS-13671